Conversation
The checklist gate counted every `- [ ]` / `- [x]` line in every checklist file, fenced blocks included. A checklist that documents the checkbox format with an example fence therefore reported unchecked items nobody can ever tick, and /speckit-implement stops on a non-zero unchecked count -- so writing down the format blocked implementation. /speckit-clarify already scopes its scan to markers outside code fences, so this was also the two commands disagreeing about what a checklist item is. They now state the same rule. Closes github#4272
The scan-instruction pattern only recognised a definition whose first marker is unchecked, so implement.md's "Checked items: Lines matching `- [X]`" line was never parametrised. Removing its code-fence exclusion left the suite green. Match a checked or unchecked first marker: four definitions are now guarded instead of three, and that removal fails.
Task IDs are local to a feature -- every tasks.md restarts at T001 -- but the
dedup matched existing issues on the bare ID. So once feature 001-auth had an
issue titled T001, running the command for 002-billing saw "T001 exists" and
skipped it. The task was never created and nothing said so, which is a silent
gap in exactly the multi-feature repos this command targets.
The canonical title now carries the feature directory basename, and a task is
skipped only when an existing issue matches both that identifier and the ID.
The ID keeps its own word boundaries inside the prefixed title, so the
\bT\d{3,}\b matching from github#2968 is unchanged.
Issues filed before the prefix existed carry a bare `T001: ...`; those are
still recognised for their own feature, so upgrading does not re-create work
that is already tracked.
Closes github#4271
The sentence added in the previous commit — "so the `\bT\d{3,}\b` matching
above is unchanged by the prefix" — reached the file with two literal U+0008
BACKSPACE bytes where `\b` was meant. My editing pipeline interpreted the
escape rather than passing it through.
A control character cannot appear in a YAML block scalar, so the generator
fell back to a double-quoted flow scalar for the whole prompt, and
`test_yaml_has_prompt` failed on `speckit.taskstoissues.yaml`:
AssertionError: speckit.taskstoissues.yaml missing prompt block scalar
Reproduced and pinned locally: with the two bytes present the test fails with
that exact message, and with them written as `\b` the goose suite is 38/38.
The rendered sentence is unchanged — it was always meant to read `\b`.
The upgrade rule treated a bare `T001: ...` title as this feature's whenever no scoped title existed for that ID. That is exactly the state of every feature on its first run after upgrading, so a bare T001 filed for 001-auth still suppressed 002-billing's T001: the github#4271 skip, back. A bare title names no feature, and neither does the absence of a scoped one, so no inference from the tracker can settle it. Skipping drops a task silently; creating duplicates one an existing user already tracks. The command now lists every ID that matched only a bare title, with the issue and this feature's description for the task, and asks before creating anything. Confirmed issues are skipped, and retitled to the scoped form only if the user agrees, so the question does not recur.
…append them Converge is meant to assess a finished implementation, and nothing enforced that. Run while tasks.md still had open work, it assessed the code anyway, found that unbuilt work as new gaps and appended it again under fresh IDs, so every re-run duplicated its own remediation tasks (github#4269). And a run that found nothing new reported `converged`, whose report says the implementation satisfies the spec while tracked work is still open. Step 1 now checks for unchecked tasks (outside code fences, the rule implement counts by) and stops before any assessment, listing them and pointing to implement, with tasks.md untouched. Once every task is checked, each task joins the intent inventory, so one marked done but not built is a finding traced to its ID. Anything appended makes the next run stop again until it is implemented. The handoff and docs/reference/agentic-sdd.md describe the gate, and the tests pin the lifecycle. Closes github#4269
|
Thanks @ntdatt812 for consolidating these related changes and incorporating the earlier feedback. The converge prerequisite follows the lifecycle I requested, and the legacy issue handling now asks rather than guessing which feature a bare task ID belongs to. The scope is suitable for review. Please complete the existing Claude Code disclosure with the model(s) and settings/mode used; the tool and extent are already documented. On my side, the next step is CI and review of this consolidated head. There’s no need to split it back into the original PRs. Drafted for @mnriem by GitHub Copilot (model: GPT-6 Astra). |
There was a problem hiding this comment.
🟡 Changes recommended
Pagination can misclassify legacy issues, and overlapping task and requirement findings can append duplicate remediation work.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Consolidates task-bookkeeping fixes across implement, taskstoissues, and converge.
Changes:
- Excludes fenced checkbox examples from task counts.
- Scopes issue deduplication by feature.
- Gates convergence on completed tasks and adds regression coverage.
File summaries
| File | Description |
|---|---|
templates/commands/implement.md |
Excludes fenced checkboxes. |
templates/commands/taskstoissues.md |
Adds feature-scoped issue titles and legacy handling. |
templates/commands/converge.md |
Adds prerequisite gate and task inventory. |
docs/reference/agentic-sdd.md |
Documents convergence lifecycle. |
tests/unit/test_checklist_scan_contract.py |
Tests checkbox scan rules. |
tests/unit/test_taskstoissues_feature_scope.py |
Tests feature-scoped deduplication. |
tests/unit/test_converge_prerequisite.py |
Tests convergence prerequisites. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - **Task inventory**: every task in `tasks.md` — all of them checked by this point — with | ||
| the work it describes and the file paths it names. A task marked done whose work is | ||
| absent from the code, or only partly there, is a finding like any other, traced to | ||
| that task's ID. |
| 1. **Fetch existing issues for deduplication**: Before creating anything, build the set of task IDs you are about to process from `tasks.md` (each is a `T` followed by **at least** three digits, e.g. `T001` — `__SPECKIT_COMMAND_CONVERGE__` assigns new IDs with `T{M+1:03d}`, which is a floor rather than a cap, so once a file has more than 999 tasks the IDs are four digits or longer). Then use the GitHub MCP server's `list_issues` tool to look for issues that already cover those IDs. Do not pass a `state` value, since omitting it makes the tool return both open and closed issues. Request `perPage: 100` to keep the number of calls down, and since the tool uses cursor-based pagination, request pages with the `after` parameter (using the `endCursor` from the previous response). For each issue title, match it against the task ID pattern `\bT\d{3,}\b` (the `{3,}` accepts four-digit and longer IDs — with `\d{3}` a title containing `T1000` would not match at all, because the trailing `\b` cannot fall between two digits, so that task would be silently neither deduplicated nor created; word boundaries still stop a token like `ST001` from matching, and force the whole digit run to be consumed so `T100` can never match inside `T1000`; this also recognises titles written as `T001 ...`, `T001: ...` or `[T001] ...`) and, when it matches one of your task IDs, mark that ID as already having an issue. Stop paginating as soon as every task ID has been matched, or when there are no more pages, so you do not keep fetching the whole repository's issue history once all task IDs are accounted for. This bounds the number of calls on repos with large issue histories and still prevents duplicates when the command is re-run after `tasks.md` is regenerated or the skill is re-invoked. | ||
| 1. For each task in the list, use the GitHub MCP server to create a new issue in the repository that is representative of the Git remote. Task lines in `tasks.md` start with a markdown checkbox, so first strip the leading `- [ ]` (and any `[P]` / `[US#]` markers) to recover the task ID and its description. Create the issue with a single canonical title of the form `T001: <description>`, with the ID written once followed by the task description (for example, the line `- [ ] T001 Create project structure` becomes the title `T001: Create project structure`). | ||
| - **Skip** any task whose ID is already present in the set of existing issues from the previous step, and report it (for example, `T001 already has an issue, skipping`). | ||
| 1. **Fetch existing issues for deduplication**: Before creating anything, build the set of task IDs you are about to process from `tasks.md` (each is a `T` followed by **at least** three digits, e.g. `T001` — `__SPECKIT_COMMAND_CONVERGE__` assigns new IDs with `T{M+1:03d}`, which is a floor rather than a cap, so once a file has more than 999 tasks the IDs are four digits or longer). Then use the GitHub MCP server's `list_issues` tool to look for issues that already cover those IDs. Do not pass a `state` value, since omitting it makes the tool return both open and closed issues. Request `perPage: 100` to keep the number of calls down, and since the tool uses cursor-based pagination, request pages with the `after` parameter (using the `endCursor` from the previous response). For each issue title, match it against the task ID pattern `\bT\d{3,}\b` (the `{3,}` accepts four-digit and longer IDs — with `\d{3}` a title containing `T1000` would not match at all, because the trailing `\b` cannot fall between two digits, so that task would be silently neither deduplicated nor created; word boundaries still stop a token like `ST001` from matching, and force the whole digit run to be consumed so `T100` can never match inside `T1000`; this also recognises titles written as `T001 ...`, `T001: ...` or `[T001] ...`) and, when it matches one of your task IDs, mark that ID as already having an issue **only if the title also carries this feature's identifier** (see below). Task IDs restart at `T001` in every feature's `tasks.md`, so an unscoped match means the first feature to reach the tracker permanently suppresses `T001` for every later feature -- a silent gap in exactly the multi-feature repos this command is for. Stop paginating as soon as every task ID has been matched, or when there are no more pages, so you do not keep fetching the whole repository's issue history once all task IDs are accounted for. This bounds the number of calls on repos with large issue histories and still prevents duplicates when the command is re-run after `tasks.md` is regenerated or the skill is re-invoked. |
Consolidates #4313, #4314 and #4330 into one pull request, as asked on #4460 (
author-over-cap). Those three will be closed in favour of this one. Each also had review feedback outstanding; that is addressed here, and noted per section below.The three share one subject: how the command templates read task bookkeeping in
tasks.mdand in the issue tracker. They stay separable. Each fix is its own run of commits, and each run was cherry-picked alone ontomainand passes its own tests there, so any one can be dropped without touching the others.Closes #4269, closes #4271, closes #4272.
1.
/speckit-implementcounts checkbox markers inside code fences (#4272), from #4313implement.mddefined its total, checked and unchecked counts on every- [ ]/- [x]line, so a checklist that documents the checkbox format in a fenced example reported items nobody can tick. A non-zero unchecked count stops implementation./speckit-clarifyalready scoped its scan to markers outside code fences;implementnow does the same.tests/unit/test_checklist_scan_contract.pyasserts that every line in a command template that defines a checkbox-marker scan also excludes code fences.Review feedback: Copilot pointed out that the test only recognised a definition whose first marker is unchecked, so the "Checked items: Lines matching
- [X]" definition was never parametrised; removing its exclusion left the suite green. Measured before changing it: with the old pattern that removal passes 4/4. The pattern now accepts a checked or unchecked first marker, four definitions are guarded instead of three, and the same removal fails.2.
/speckit-taskstoissuesdedup matches bare task IDs across features (#4271), from #4314Task IDs restart at
T001in every feature'stasks.md, and dedup matched existing issues on the ID alone. So the first feature to reach the tracker permanently suppressedT001for every later feature, and those tasks were silently never created. Issue titles now carry the feature ([002-billing] T001: ..., from theFEATURE_DIRbasename), and a task is skipped only when an issue matches both the feature and the ID.Review feedback, and backward compatibility: Copilot found that the upgrade rule brought the bug straight back. It treated a bare legacy
T001: ...title as this feature's whenever no scoped title existed for that ID, and that is the state of every feature on its first run after upgrading: a bareT001filed for001-authstill suppressed002-billing'sT001.A bare title names no feature, and neither does the absence of a scoped one, so nothing in the tracker can settle which feature it belongs to. Deciding either way breaks someone: skipping drops a task silently, and creating duplicates one an existing user already tracks. That second case is the compatibility concern raised on #4314. So the command now lists every ID that matched only a bare title, each with the issue and this feature's description for the task, and asks before creating anything. Confirmed issues are skipped. It then offers to retitle them to the scoped form, and does so only for the ones the user agrees to, so the question does not come back on later runs. Existing users therefore get no duplicates and no silent gaps; the cost is one question per legacy issue, once.
The U+0008 bytes Copilot flagged on the earlier commit were already fixed in that PR (
\bwritten as two characters);tests/integrations/test_integration_goose.py, which caught them, passes.3.
/speckit-convergeis not idempotent (#4269), from #4330#4330 dropped findings that an unchecked task already covered. Review there identified the root cause: converge never enforced its prerequisite. Run while
tasks.mdstill had open tasks, it assessed the code anyway, found that unbuilt work as new gaps and appended it again under fresh IDs. And a run whose findings were all already tracked ended inconverged, whose report says the implementation satisfies the spec while tracked work is still open.Reworked around the lifecycle instead of the dedup rule:
tasks.mdhas any unchecked task (outside code fences, the ruleimplementcounts by), converge stops before assessing anything. It lists the open task IDs, points to/speckit-implement, and leavestasks.mdbyte-for-byte unchanged. This is neitherconvergednortasks_appended.convergedis only reachable once the work is actually finished.docs/reference/agentic-sdd.mddescribe the gate.tests/unit/test_converge_prerequisite.pypins the lifecycle: the gate sits in Step 1 and stops before Step 2, namesimplement, lists the open IDs, leavestasks.mduntouched, counts outside code fences, is not reported asconverged, the inventory includes tasks, and the docs describe it. Copilot's other note on #4330, that the dedup test did not pin substance-based matching, no longer applies, since that rule is gone.Evidence
maintest_checklist_scan_contract.pytest_taskstoissues_feature_scope.pytest_converge_prerequisite.pymain: its own tests plus the goose integration tests pass (43, 43 and 45).STOPfails 1; removing the task inventory fails 1.uvx ruff@0.15.0 check src tests: clean. markdownlint reports the same number of findings on the four touched Markdown files as onmain(all pre-existing, in untouched lines).tests/unitandtests/integrationson Windows: the only failures are symlink-privilege errors (WinError 1314); the two whose names do not say so fail identically on a clean checkout ofmain.AI disclosure
Per CONTRIBUTING: this pull request (code, tests, measurements and this description) was developed with Claude Code as a coding agent.